Skip to content

v8: add setHeapProfileNearHeapLimit - #64676

Open
IlyasShabi wants to merge 8 commits into
nodejs:mainfrom
IlyasShabi:ishabi/near-heap-limit-profile
Open

v8: add setHeapProfileNearHeapLimit#64676
IlyasShabi wants to merge 8 commits into
nodejs:mainfrom
IlyasShabi:ishabi/near-heap-limit-profile

Conversation

@IlyasShabi

@IlyasShabi IlyasShabi commented Jul 22, 2026

Copy link
Copy Markdown
Member

This PR adds the v8.setHeapProfileNearHeapLimit(limit) API, which writes the active sampling heap profile to disk when V8 approaches the heap limit. It complements v8.setHeapSnapshotNearHeapLimit() use cases that need a sampled allocation profile rather than a full heap snapshot.

Heap profiling must first be enabled using v8.startHeapProfile() or the --heap-prof CLI option.

v8.setHeapProfileNearHeapLimit() and v8.setHeapSnapshotNearHeapLimit() can be active on the same isolate.

Signed-off-by: ishabi <ilyasshabi94@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 22, 2026
@IlyasShabi
IlyasShabi marked this pull request as ready for review July 22, 2026 14:03
Comment thread src/env.cc Outdated
env,
});
console.log(child.stdout.toString());
console.log(child.stderr.toString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the console.log(...) log statements necessary for the test? If not, can we remove them? If they are, can you add a comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment thread doc/api/v8.md
@joyeecheung

joyeecheung commented Jul 22, 2026

Copy link
Copy Markdown
Member

I struggle to see how this API can ever stabilize - what one can do when the heap is near the heap limit is very limited, and in particular if you do too much in the callback to incur another allocation, the program would be in a very unstable state. Our current callbacks only do very limited things within the C++, allowing users to do arbitrary things in JS sounds like an API that invites bugs...I think a safer option would be to only write a profile when it reaches the limit, though even then I am not sure how well the profiler works when the heap is about to reach the limit...

@legendecas

Copy link
Copy Markdown
Member

I'm also wary on adding a JS callback to be invoked at new heap limit. This callback also takes a heap profile parameter which is taken on a big heap, sounds like exacerbating the problem it wants to fix.

Comment thread doc/api/v8.md Outdated

v8.startHeapProfile();
v8.setHeapProfileNearHeapLimit((profile) => {
console.log(JSON.parse(profile));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example asks users to exacerbating the heap usages when the heap is near limit...

@IlyasShabi

Copy link
Copy Markdown
Member Author

This API is not intended to make JS safe near OOM. It closes a blind spot in the sampling heap profiler: today, if a process starts startHeapProfile() and reaches a fatal OOM, the accumulated sampling data is lost because stopHeapProfile() is never called so the API provides a best-effort chance to retrieve that data before the process terminates.
The samples are collected continuously, so the near-limit event does not take a full heap snapshot or walk every heap object. However profiler->GetAllocationProfile() still materializes the sampled call tree and may allocate, so capture itself is also best-effort.

@joyeecheung

Copy link
Copy Markdown
Member

Why would the application want to save the profile in JS memory when it's likely not relevent for the application itself that may already crash soon? I think a file-backed API would achieve the same thing without making the users waste the remaining memory with (arguably very noisy) diagnostic data.

@IlyasShabi
IlyasShabi requested a review from Qard July 23, 2026 09:46
@IlyasShabi

Copy link
Copy Markdown
Member Author

@joyeecheung Yes I agree with your proposal! I changed the approach to a file-backed one. please feel free to review it

@IlyasShabi
IlyasShabi requested a review from legendecas July 27, 2026 09:42
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 49.60630% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (4bec191) to head (481ece9).
⚠️ Report is 75 commits behind head on main.

Files with missing lines Patch % Lines
src/env.cc 20.27% 56 Missing and 3 partials ⚠️
src/env-inl.h 92.00% 0 Missing and 2 partials ⚠️
src/node_v8.cc 84.61% 0 Missing and 2 partials ⚠️
src/node_worker.cc 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64676      +/-   ##
==========================================
+ Coverage   90.13%   90.14%   +0.01%     
==========================================
  Files         741      744       +3     
  Lines      242106   242641     +535     
  Branches    45551    45709     +158     
==========================================
+ Hits       218211   218728     +517     
+ Misses      15430    15411      -19     
- Partials     8465     8502      +37     
Files with missing lines Coverage Δ
lib/v8.js 98.79% <100.00%> (+0.02%) ⬆️
src/env.h 98.21% <ø> (ø)
src/node_profiling.cc 81.08% <ø> (-0.26%) ⬇️
src/node_worker.cc 82.07% <66.66%> (+0.25%) ⬆️
src/env-inl.h 94.81% <92.00%> (-0.18%) ⬇️
src/node_v8.cc 88.22% <84.61%> (-0.07%) ⬇️
src/env.cc 82.15% <20.27%> (-3.39%) ⬇️

... and 98 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@IlyasShabi
IlyasShabi requested a review from joyeecheung July 28, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants